Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(protocol-designer): fix long step name, step details, and step summary layout issue #16547

Merged
merged 9 commits into from
Oct 24, 2024

Conversation

syao1226
Copy link
Collaborator

@syao1226 syao1226 commented Oct 21, 2024

fixes RQA-3355

Overview

Fixing layout issues in StepFormToolbox, StepContainer, and StepSummary when step names, step details, and step summaries are too long, causing the step icon to shrink, the step name and step summaries content to go off-screen, and the step details text to exceed the window size.

Refer to the ticket for details on the current layout issues and the expected design.

Test Plan and Hands on Testing

Changelog

Review requests

Risk assessment

@syao1226 syao1226 changed the title fix(protocol-designer): fix long step name layout issue fix(protocol-designer): fix long step name and step details layout issue Oct 22, 2024
@syao1226 syao1226 marked this pull request as ready for review October 22, 2024 20:31
@syao1226 syao1226 requested a review from a team as a code owner October 22, 2024 20:31
@@ -209,8 +210,8 @@ export function StepFormToolbox(props: StepFormToolboxProps): JSX.Element {
height="calc(100vh - 64px)"
title={
<Flex gridGap={SPACING.spacing8} alignItems={ALIGN_CENTER}>
<Icon size="1rem" name={icon} />
<StyledText desktopStyle="bodyLargeSemiBold">
<Icon size="1rem" name={icon} style={{ flexShrink: 0 }} />
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this works as expected

the following also works

minWidth="1rem"

cc @jerader @ncdiehl11

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ohh didn't take that into consideration, i'll work on fixing that as well! thanks for the catch, alex.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'll change it to use minWidth, it looks cleaner to me. thanks, koji

@koji
Copy link
Contributor

koji commented Oct 23, 2024

@syao1226 I just merged my PR that includes line clamp style in atoms into edge.

import LINE_CLAMP_TEXT_STYLE from atoms and add css={LINE_CLAMP_TEXT_STYLE(max_lines)} to a tag.

desktopStyle="bodyLargeSemiBold"
css={`
${LINE_CLAMP_TEXT_STYLE(2)}
word-break: break-all
Copy link
Contributor

@koji koji Oct 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

word-wrap: break-word; didn't work?

Copy link
Collaborator Author

@syao1226 syao1226 Oct 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i added break-all to handle case of very long words or words connected by symbols like underscore.
Screenshot 2024-10-23 at 1 53 46 PM
Screenshot 2024-10-23 at 1 54 49 PM

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks!

Copy link
Contributor

@koji koji left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@syao1226 syao1226 changed the title fix(protocol-designer): fix long step name and step details layout issue fix(protocol-designer): fix long step name, step details, and step summary layout issue Oct 23, 2024
@syao1226 syao1226 requested a review from koji October 24, 2024 14:52
Copy link
Contributor

@koji koji left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@syao1226 syao1226 merged commit 1734ff5 into edge Oct 24, 2024
14 checks passed
@syao1226 syao1226 deleted the protocol_designer-fix-step-name branch October 24, 2024 17:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants